Skip to content

fix(dashboard): show loading indicator when uploading assets#4940

Open
latifniz wants to merge 1 commit into
vendurehq:masterfrom
latifniz:fix/asset-upload-progress-indicator
Open

fix(dashboard): show loading indicator when uploading assets#4940
latifniz wants to merge 1 commit into
vendurehq:masterfrom
latifniz:fix/asset-upload-progress-indicator

Conversation

@latifniz

@latifniz latifniz commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

When uploading assets via Catalog → Assets, there was zero visual feedback during the upload process. The UI appeared completely frozen until all files finished uploading, then they appeared all at once. This was confusing especially when uploading multiple large files.

What changed

Added upload progress feedback to the AssetGallery component:

  • The Upload button shows a spinner and becomes disabled while upload is in progress, preventing accidental double-submissions
  • A semi-transparent overlay with a spinner and "Uploading assets..." message appears over the gallery grid during upload
  • Once the upload completes the overlay disappears and the gallery refreshes automatically

Root cause

useMutation from React Query exposes an isPending state but it was never being used. The fix simply destructures isPending and wires it up to the UI.

Notes

True per-file progress (e.g. percentage bar) is not possible with the current architecture because api.mutate uses the Fetch API which does not expose upload progress events. This fix gives the best feedback possible without rewriting the upload mechanism to use XHR.

The overlay also shows in the asset picker dialog (used when selecting assets on product/category pages) which is correct behaviour.

Checklist

  • I have set a clear title
  • My PR is small and contains a single change
  • I have checked my own PR

Fixes issue #4936

here is video after fix ( this is local host so it takes less than a second but loader is visible, the issue i reported was in prod so it took a lot of time , see video in the issue as well)

fixloader.mp4

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
vendure-storybook Ready Ready Preview, Comment Jul 10, 2026 9:26am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Channel switching now preserves the current route when navigating between channels, with fallback routing when no matching target route exists. Asset uploads expose pending state, disable the upload button, show a loading icon, and display an upload overlay while processing.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding loading feedback for asset uploads in the dashboard.
Description check ✅ Passed The description includes the change summary, related issue, root cause, notes, and checklist, so it is mostly complete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx (1)

249-254: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add an onError handler to the useMutation call.

The mutation only defines onSuccess but no onError. As per coding guidelines, useMutation calls should include both onSuccess and onError handlers. Without onError, upload failures are silently swallowed — the spinner stops but the user gets no feedback that the upload failed.

♻️ Proposed fix
 const { mutate: createAssets, isPending: isUploading } = useMutation({
     mutationFn: api.mutate(createAssetsDocument),
     onSuccess: () => {
         queryClient.invalidateQueries({ queryKey });
     },
+    onError: (error) => {
+        // Show error notification to the user
+        console.error('Asset upload failed:', error);
+    },
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx` around
lines 249 - 254, Add an onError handler to the useMutation call creating
createAssets in the asset gallery, using the existing notification or
error-reporting pattern to provide clear user feedback when asset upload fails
while preserving the current onSuccess invalidation behavior.

Source: Coding guidelines

🧹 Nitpick comments (1)
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx (1)

447-452: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add role="status" to the upload overlay for screen reader accessibility.

The overlay visually communicates upload progress but lacks an ARIA live region. Screen reader users won't be notified when the upload starts or completes. Adding role="status" (which implies aria-live="polite") addresses this with minimal effort.

♿ Proposed fix
 {isUploading && (
-    <div className="absolute inset-0 bg-background/70 backdrop-blur-sm z-10 flex flex-col items-center justify-center rounded-md">
+    <div role="status" className="absolute inset-0 bg-background/70 backdrop-blur-sm z-10 flex flex-col items-center justify-center rounded-md">
         <Loader2 className="h-10 w-10 text-primary animate-spin mb-3" />
         <p className="text-center font-medium"><Trans>Uploading assets...</Trans></p>
     </div>
 )}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx` around
lines 447 - 452, Add role="status" to the upload overlay div rendered by the
isUploading condition in the asset gallery, so screen readers announce upload
state changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx`:
- Around line 249-254: Add an onError handler to the useMutation call creating
createAssets in the asset gallery, using the existing notification or
error-reporting pattern to provide clear user feedback when asset upload fails
while preserving the current onSuccess invalidation behavior.

---

Nitpick comments:
In `@packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx`:
- Around line 447-452: Add role="status" to the upload overlay div rendered by
the isUploading condition in the asset gallery, so screen readers announce
upload state changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8b1632cf-e9cb-4aee-b6d1-525a04a1c3ef

📥 Commits

Reviewing files that changed from the base of the PR and between ba5d189 and 1b37f51.

📒 Files selected for processing (2)
  • packages/dashboard/src/lib/components/layout/channel-switcher.tsx
  • packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx

@grolmus grolmus left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice UX improvement — the disabled button + spinner + overlay reads well, and thanks for sticking to semantic tokens (bg-background, text-primary). Two things before this can merge:

  1. dashboard i18n sync is red — the new <Trans>Uploading assets…</Trans> string hasn't been extracted into the translation catalogs. From packages/dashboard, run:
    bun run i18n:extract
    
    (or npm run i18n:extract --workspace=@vendure/dashboard from the repo root), then commit the updated packages/dashboard/src/i18n/locales/*.po files. The new message will land untranslated in the non-English catalogs — that's expected; the check just needs the catalogs to match lingui extract output.
  2. The branch is ~32 commits behind master — please merge the latest master in (the "Update branch" button on this PR is the easiest way) so CI runs against current code.

Heads-up: #4941 also edits asset-gallery.tsx, so whichever of the two merges second will need a quick rebase.

Once i18n is green this LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants